-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use intra-doc links in core::iter module #74300
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
This would result in dead links in std due to #65983, also see #73579 (comment) where I personally first noticed this problem. |
/// [`sum`]: ../../std/iter/trait.Sum.html#tymethod.sum | ||
/// [`FromIterator`]: ../../std/iter/trait.FromIterator.html | ||
/// [`Iterator::sum`]: ../../std/iter/trait.Iterator.html#method.sum | ||
/// [`sum`]: #tymethod.sum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this not have an equivalent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fragment link is fine as the sum
method referred belongs to Sum trait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this not be Sum::sum
or Self::sum
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it documented in Intra RFC? That's new to me!
I'm pretty sure |
As a side note, #73101 is almost ready to be merged, maybe 1 or 2 more weeks. |
Checked locally and ran linkchecker. I don't see any broken links |
|
After experimenting, I observed two things:
The broke links are detected by linkchecker. I don't know if there are more empty-links, |
#73101 just landed so this should be fixed once you rebase 😄 You'll need to use |
Rebased. Looks like stage1 rustdoc fixes the Option link in core::iter . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! It's good to see how this is used in the wild, it gave me ideas for more features to add 😆 I had one small nit but everything else is informative or notes to myself.
/// [`Option<T>`]: ../../std/option/enum.Option.html | ||
/// [`Some`]: ../../std/option/enum.Option.html#variant.Some | ||
/// [`None`]: ../../std/option/enum.Option.html#variant.None | ||
/// [`Option<T>`]: Option |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is #62834. This is a good workaround in the meantime though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why not just use Option
rather than Option<T>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Option<T>
seems to be clearer for beginners/first readers of Iterator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use Option<T>
in the book, but I don't think beginners get here easily.
@@ -1366,8 +1350,7 @@ pub trait Iterator { | |||
/// [`Some(T)`] again. `fuse()` adapts an iterator, ensuring that after a | |||
/// [`None`] is given, it will always return [`None`] forever. | |||
/// | |||
/// [`None`]: ../../std/option/enum.Option.html#variant.None | |||
/// [`Some(T)`]: ../../std/option/enum.Option.html#variant.Some | |||
/// [`Some(T)`]: Some |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen this quite a few times now, maybe it would be a good feature for intra-doc links to add, same as #62834?
#74185 (comment). Maybe we should add |
Because the old one is harder to read and confuse typing checkers.
It turns out this will be on by default starting in #71670. But it doesn't hurt to have it on now. |
r=me, but I don't have bors permissions until rust-lang/team#384 lands. |
@bors r+ rollup |
@jyn514: 🔑 Insufficient privileges: Not in reviewers |
@bors r=jyn514 rollup=always |
📌 Commit 5ffdd7c has been approved by |
Use intra-doc links in core::iter module This will make core::iter doc depend less on std doc.
…arth Rollup of 18 pull requests Successful merges: - rust-lang#71670 (Enforce even more the code blocks attributes check through rustdoc) - rust-lang#73930 (Make some Option methods const) - rust-lang#74009 (Fix MinGW `run-make-fulldeps` tests) - rust-lang#74056 (Add Arguments::as_str().) - rust-lang#74169 (Stop processing unreachable blocks when solving dataflow) - rust-lang#74251 (Teach bootstrap about target files vs target triples) - rust-lang#74288 (Fix src/test/run-make/static-pie/test-aslr.rs) - rust-lang#74300 (Use intra-doc links in core::iter module) - rust-lang#74364 (add lazy normalization regression tests) - rust-lang#74368 (Add CSS tidy check) - rust-lang#74394 (Remove leftover from emscripten fastcomp support) - rust-lang#74411 (Don't assign `()` to `!` MIR locals) - rust-lang#74416 (Use an UTF-8 locale for the linker.) - rust-lang#74424 (Move hir::Place to librustc_middle/hir) - rust-lang#74428 (docs: better demonstrate that None values are skipped as many times a…) - rust-lang#74438 (warn about uninitialized multi-variant enums) - rust-lang#74440 (Fix Arc::as_ptr docs) - rust-lang#74452 (intra-doc links: resolve modules in the type namespace) Failed merges: r? @ghost
This will make core::iter doc depend less on std doc.